Skip to content

web: non-blocking web_server, browser console mirroring, Tcl 9 fix#10392

Open
maliberty wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:web-non-blocking
Open

web: non-blocking web_server, browser console mirroring, Tcl 9 fix#10392
maliberty wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:web-non-blocking

Conversation

@maliberty
Copy link
Copy Markdown
Member

Summary

Make web_server a transition point in scripts and a blocking suspend-the-prompt call at the interactive Tcl prompt, with the rest of the script running through the now-active server and browser-driven commands serializing against the main thread.

  • web_server Tcl proc: walks info frame for tclreadline::Loop to detect interactive vs. script context. Scripts return immediately so subsequent lines run as if typed in the browser console; interactive use calls web_server_wait_cmd which parks the main thread in Tcl_DoOneEvent so the launching terminal prompt is suppressed while the server is up. The event mask excludes TCL_FILE_EVENTS so tclreadline's stdin handler does not keep reading the terminal.

  • Tcl 9 cross-thread fix: TclEvaluator::eval marshals browser worker tcl_eval requests to the main thread via Tcl_ThreadQueueEvent + condition variable instead of calling Tcl_Eval directly on the worker. Tcl 9 isolates per-thread interpreter state, so a direct worker Tcl_Eval fails to see globals set on the main thread (e.g. set x 1 at the prompt followed by puts $x in the browser returns an unset-variable error). Evaluation uses Tcl_EvalEx(..., TCL_EVAL_GLOBAL) so commands run at the top-level namespace regardless of the Tcl call frame.

  • Browser console mirroring: a level-1 Tcl_CreateObjTrace broadcasts every main-thread top-level command as a console_input push frame, so script lines and terminal-typed commands appear in the browser Tcl console as >>> cmd. The trace filters by Tcl_GetCurrentThread() == mainThreadId() to avoid double-echoing browser-typed commands (the JS widget echoes them locally). Two Tcl_StackChannel transforms on stdout / stderr forward puts/error output through to the terminal and broadcast it to the browser as a log frame.

  • Browser-typed exit / web_server -stop wake the main thread via a no-op event queued with Tcl_ThreadQueueEvent + Tcl_ThreadAlert. The wait loop checks exitRequested() / stopRequested() and the calling site (Main.cc or web_server_wait_cmd) calls stop() and, on exit-request, std::exit(0) on the main thread.

  • Channel mirror passes the actual written count (not requested) to the broadcast so a partial write doesn't claim bytes that never reached the terminal.

  • WebServer::stop() integrates the new teardown (unstack channels, remove trace, restore exit, reset state) and remains callable from any thread via stopAndJoinIoThreads's self-join handling.

  • main.js renders console_input push frames as >>> cmd lines.

Tests adapted: TclEvaluator constructor now takes a shared std::mutex& and Tcl_ThreadId; tests pass nullptr for the thread id so the direct fast-path runs.

Type of Change

  • New feature

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • [ x I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Make `web_server` a transition point in scripts and a blocking
suspend-the-prompt call at the interactive Tcl prompt, with the rest
of the script running through the now-active server and browser-driven
commands serializing against the main thread.

* `web_server` Tcl proc: walks `info frame` for `tclreadline::Loop`
  to detect interactive vs. script context.  Scripts return
  immediately so subsequent lines run as if typed in the browser
  console; interactive use calls `web_server_wait_cmd` which parks
  the main thread in `Tcl_DoOneEvent` so the launching terminal
  prompt is suppressed while the server is up.  The event mask
  excludes `TCL_FILE_EVENTS` so tclreadline's stdin handler does
  not keep reading the terminal.

* Tcl 9 cross-thread fix: `TclEvaluator::eval` marshals browser
  worker `tcl_eval` requests to the main thread via
  `Tcl_ThreadQueueEvent` + condition variable instead of calling
  `Tcl_Eval` directly on the worker.  Tcl 9 isolates per-thread
  interpreter state, so a direct worker `Tcl_Eval` fails to see
  globals set on the main thread (e.g. `set x 1` at the prompt
  followed by `puts $x` in the browser returns an unset-variable
  error).  Evaluation uses `Tcl_EvalEx(..., TCL_EVAL_GLOBAL)` so
  commands run at the top-level namespace regardless of the
  Tcl call frame.

* Browser console mirroring: a level-1 `Tcl_CreateObjTrace`
  broadcasts every main-thread top-level command as a
  `console_input` push frame, so script lines and terminal-typed
  commands appear in the browser Tcl console as `>>> cmd`.  The
  trace filters by `Tcl_GetCurrentThread() == mainThreadId()` to
  avoid double-echoing browser-typed commands (the JS widget
  echoes them locally).  Two `Tcl_StackChannel` transforms on
  stdout / stderr forward `puts`/error output through to the
  terminal and broadcast it to the browser as a `log` frame.

* Browser-typed `exit` / `web_server -stop` wake the main thread
  via a no-op event queued with `Tcl_ThreadQueueEvent` +
  `Tcl_ThreadAlert`.  The wait loop checks `exitRequested()` /
  `stopRequested()` and the calling site (`Main.cc` or
  `web_server_wait_cmd`) calls `stop()` and, on exit-request,
  `std::exit(0)` on the main thread.

* Channel mirror passes the actual `written` count (not
  requested) to the broadcast so a partial write doesn't claim
  bytes that never reached the terminal.

* `WebServer::stop()` integrates the new teardown (unstack
  channels, remove trace, restore `exit`, reset state) and
  remains callable from any thread via `stopAndJoinIoThreads`'s
  self-join handling.

* main.js renders `console_input` push frames as `>>> cmd` lines.

Tests adapted: `TclEvaluator` constructor now takes a shared
`std::mutex&` and `Tcl_ThreadId`; tests pass `nullptr` for the
thread id so the direct fast-path runs.

Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
@maliberty maliberty self-assigned this May 11, 2026
@maliberty maliberty requested a review from gadfort May 11, 2026 06:43
@github-actions
Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 07954badef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/web/src/web.tcl
Comment on lines +54 to +56
if { $in_readline } {
web::web_server_wait_cmd
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep web_server blocking while scripts mutate shared state

Returning immediately in script mode means the main Tcl thread can keep running design-mutating commands while web worker threads continue servicing requests, but the worker-side serialization (tcl_eval_->mutex in handlers such as SelectHandler::handleSelect) does not protect against concurrent main-thread STA/DB access. This reintroduces unsynchronized cross-thread access to components explicitly marked non-thread-safe and can cause races or crashes when a script runs after web_server and a browser is connected.

Useful? React with 👍 / 👎.

Comment thread src/web/src/web_serve.cpp
Comment on lines +568 to +569
while (!exitRequested() && !stopRequested() && isRunning()) {
Tcl_DoOneEvent(kEventMask);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop spinning when Tcl_DoOneEvent reports no work

This loop ignores the return value of Tcl_DoOneEvent; when there are no eligible timer/idle/window handlers, Tcl can return 0 immediately instead of blocking, so the loop busy-spins until stopRequested/exitRequested flips. In headless web sessions this can peg a CPU core while waiting for user actions.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the web server's integration with the Tcl event loop to support Tcl 9's thread isolation and improve the interactive experience. Key changes include implementing cross-thread marshalling for Tcl commands via Tcl_ThreadQueueEvent, mirroring Tcl's stdout and stderr channels to the browser console using Tcl_StackChannel, and broadcasting top-level main-thread commands to connected clients. The WebServer API was updated to replace the blocking waitForStop with a Tcl-native event loop in runEventLoopUntilStop. Feedback identified a signature mismatch in the Tcl channel output procedure where a parameter should be char* instead of const char* to comply with the Tcl API.

Comment thread src/web/src/web_serve.cpp
Comment on lines +148 to +161
static int webChannelOutputProc(ClientData instanceData,
const char* buf,
int toWrite,
int* errorCodePtr)
{
auto* m = static_cast<WebChannelMirror*>(instanceData);

// Forward to the next channel in the stack so the terminal still
// sees the output.
const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent);
Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type);
ClientData parent_data = Tcl_GetChannelInstanceData(m->parent);
const int written = parent_output(
parent_data, const_cast<char*>(buf), toWrite, errorCodePtr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The signature of webChannelOutputProc does not match the Tcl_DriverOutputProc type expected by Tcl_ChannelType.outputProc. The buf parameter should be char*, not const char*.

This mismatch can cause compilation errors and relies on unsafe type conversions. Correcting the signature to char* buf aligns with the Tcl API, improves type safety, and removes the need for const_cast when calling the parent channel's output procedure.

Suggested change
static int webChannelOutputProc(ClientData instanceData,
const char* buf,
int toWrite,
int* errorCodePtr)
{
auto* m = static_cast<WebChannelMirror*>(instanceData);
// Forward to the next channel in the stack so the terminal still
// sees the output.
const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent);
Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type);
ClientData parent_data = Tcl_GetChannelInstanceData(m->parent);
const int written = parent_output(
parent_data, const_cast<char*>(buf), toWrite, errorCodePtr);
static int webChannelOutputProc(ClientData instanceData,
char* buf,
int toWrite,
int* errorCodePtr)
{
auto* m = static_cast<WebChannelMirror*>(instanceData);
// Forward to the next channel in the stack so the terminal still
// sees the output.
const Tcl_ChannelType* parent_type = Tcl_GetChannelType(m->parent);
Tcl_DriverOutputProc* parent_output = Tcl_ChannelOutputProc(parent_type);
ClientData parent_data = Tcl_GetChannelInstanceData(m->parent);
const int written = parent_output(
parent_data, buf, toWrite, errorCodePtr);
References
  1. API usability and compatibility with external driver signatures (like Tcl_DriverOutputProc) can be prioritized over strict const-correctness to avoid unsafe type conversions and ensure correct linkage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant